feat(channel): Manage knative eventing channels #967

navidshaikh · 2020-08-03T15:04:28Z

Description

Manage knative eventing channels
Use the default configured channel type
Channel type aliases via kn config and alias for InMemoryChannel
- User can now configure channel type aliases in kn config and specify
  alias to GVK mappings for easy reference on CLI and refer with --type flag
- User can also use inbuilt alias 'imc' for InMemoryChannel

Changes

Add create, delete, describe and list for channels

Reference

Fixes #954

TODO:

add configurable channel type
add an alias 'imc' for inbuilt channel type InMemoryChannel
add unit tests
add e2e tests

/lint

knative-prow-robot · 2020-08-03T15:04:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: navidshaikh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [navidshaikh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

knative-prow-robot

@navidshaikh: 7 warnings.

In response to this:

Description

Manage knative eventing channels

Use the default configured channel type

Changes

Add create, delete, describe and list for channels

Reference

Fixes #954

TODO:

add configurable channel type

add unit tests

add e2e tests

/lint

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

pkg/kn/commands/flags/subscriber.go

pkg/kn/commands/channel/channel.go

pkg/kn/commands/channel/create.go

pkg/sources/v1beta1/messaging_client.go

pkg/kn/commands/channel/flags.go

navidshaikh · 2020-08-03T15:04:43Z

/hold

for adding tests and channel type

pkg/kn/commands/channel/describe.go

navidshaikh · 2020-08-17T08:05:25Z

/hold cancel

navidshaikh · 2020-08-17T09:15:25Z

/retest

    result_collector.go:75: ERROR: Error: ReconcileIngressFailed: Ingress reconciliation failed

navidshaikh · 2020-08-17T11:10:05Z

/retest

  result_collector.go:75: ERROR: Error: cannot delete broker 'foo2' in namespace 'kne2etests19' because: timeout: broker 'foo2' not ready after 600 seconds

navidshaikh · 2020-08-17T15:17:28Z

/retest

getting ?? backingchannelready 1s backingchannelnotconfigured error, I tried running this locally with eventing nightly and it worked fine for me, giving it another retest before digging further.

- Support specifying the type of channel to create using --type. - Default is to use messaging layer configuration for channels - Channel type aliases via kn config and alias for InMemoryChannel - User can now configure channel type aliases in kn config and specify alias to GVK mappings for easy reference on CLI and refer with `--type` flag - User can also use inbuilt alias 'imc' for InMemoryChannel

navidshaikh · 2020-08-18T09:41:03Z

pkg/kn/flags/channel_types.go

+var ctypeMappings = map[string]schema.GroupVersionKind{
+	"imc": {
+		Group:   "messaging.knative.dev",
+		Version: "v1beta1",
+		Kind:    "InMemoryChannel",
+	},
+}


We'll ship channel management with kn in 0.17. We're working with v1beta1 APIs/clientset of eventing in client, to be compatible with 0.17 and earlier releases.
We create channels and defer the type to cluster/namespace defaults unless specified explicitly on cli using --type flag.
For convenience, we're giving this short hand alias for IMC channels, to override if user wants to, and pinning its GVK at v1beta1.
Looking at knative/eventing#3793 and if the cluster default is going to be IMC at v1 for 0.17, should we rename this alias and add another at v1 ?
like imcv1 and imcv1beta1, this way user get to choose whichever is available in the cluster

wdyt @matzew @rhuss

adding explicit aliases: imcv1 and imcv1beta1

what's about having a --type imc alias for v1 and --type imc:v1beta1 for v1beta1 (and for completeness sake also a --type imc:v1 ?)

reasoning behind this is, that I think we should separate the version here more visually and also make it optional so that we have a short default. I expect that v1beta1 is only temporary and will fade away over time ...

navidshaikh · 2020-08-18T09:55:39Z

/test pull-knative-client-integration-tests

result_collector.go:75: ERROR: Error: cannot delete broker 'foo2' in namespace 'kne2etests16' because: timeout: broker 'foo2' not ready after 600 seconds

maximilien

Some minor comments. The main issue I have is lack of e2e tests. Not clear how to put it all together? Perhaps a basic workflow using channels? Or even a small bash tests to show how it all hangs together?

maximilien · 2020-08-24T21:51:47Z

docs/cmd/kn_channel_create.md

+
+  # Create a channel 'imc1' of type InMemoryChannel using inbuilt alias 'imc'
+  kn channel create imc1 --type imc
+  # same as above without using inbuilt alias but providing explicit GVK


Any other way to explain this without using acronyms or simpler terms. thinking of users not familiar with the jargons.. :)

docs/cmd/kn_channel_create.md

docs/cmd/kn_channel_delete.md

docs/cmd/kn_channel_describe.md

pkg/kn/commands/channel/list.go

rhuss · 2020-08-25T09:19:58Z

@navidshaikh can we tackle this still today to get this into the release for today ? that would be super cool.

navidshaikh · 2020-08-25T11:05:19Z

The main issue I have is lack of e2e tests. N

@maximilien : The PR already has e2e tests (see the PR description check-list), test/e2e/channels_test.go

navidshaikh · 2020-08-25T11:08:11Z

can we tackle this still today to get this into the release for today ? that would be super cool.

@rhuss : I've answered Max's question and resolved them. I think the only outstanding query I'd was whether to have version in IMC channel alias that we ship. For now its imc, my suggestion is to go with imcv1 and imcv1beta1 so users connected to latest evening setup or earlier one could choose accordingly.

#967 (comment)

navidshaikh · 2020-08-25T12:16:15Z

/retest

seems like infra issue

rhuss · 2020-08-25T14:03:23Z

pkg/kn/flags/channel_types.go

+		Version: "v1beta1",
+		Kind:    "InMemoryChannel",
+	},
+	"imcv1": {


i see, colons are already used for gvk kind of specification. so i'm happy to use imcv1beta1 for the 'old' imc but please use 'imc' for the v1 as I expect this to be used much more often.

@rhuss : fixed, PTAL.

pkg/kn/commands/channel/channel.go

pkg/kn/commands/channel/channel_test.go

pkg/kn/commands/channel/create.go

pkg/kn/commands/channel/create_test.go

pkg/kn/commands/channel/delete.go

pkg/kn/commands/channel/list_test.go

pkg/kn/flags/channel_types.go

pkg/messaging/v1beta1/channels_client.go

pkg/messaging/v1beta1/channels_client_mock.go

pkg/messaging/v1beta1/client.go

knative-metrics-robot · 2020-08-25T15:10:56Z

The following is the coverage report on the affected files.
Say /test pull-knative-client-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/errors/factory.go	95.7%	96.0%	0.3
pkg/kn/commands/channel/channel.go	Do not exist	57.1%
pkg/kn/commands/channel/create.go	Do not exist	88.0%
pkg/kn/commands/channel/delete.go	Do not exist	92.9%
pkg/kn/commands/channel/describe.go	Do not exist	76.9%
pkg/kn/commands/channel/flags.go	Do not exist	53.5%
pkg/kn/commands/channel/list.go	Do not exist	80.0%
pkg/kn/commands/types.go	53.8%	55.9%	2.1
pkg/kn/config/config.go	75.0%	75.3%	0.3
pkg/kn/flags/channel_types.go	Do not exist	91.7%

navidshaikh · 2020-08-25T15:53:31Z

/retest

 result_collector.go:75: ERROR: Error: ReconcileIngressFailed: Ingress reconciliation failed

rhuss

thanks !

/lgtm

* feat(channel): Manage knative eventing channels - Support specifying the type of channel to create using --type. - Default is to use messaging layer configuration for channels - Channel type aliases via kn config and alias for InMemoryChannel - User can now configure channel type aliases in kn config and specify alias to GVK mappings for easy reference on CLI and refer with `--type` flag - User can also use inbuilt alias 'imc' for InMemoryChannel * Let channel reconcile, sleep for 5 secs after creation * Add imcv1 and imcv1beta1 aliases * Rename imcv1 alias to imc

…1575) (knative#967) * Fix for file not found error message discrepancy in windows * Added comment Co-authored-by: Gunjan Vyas <[email protected]>

Fix for file not found error message discrepancy in windows (knative#1575) (knative#967) Co-authored-by: Gunjan Vyas <[email protected]> change display versions (knative#1601) (knative#985) Co-authored-by: kobayashi <[email protected]>

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Aug 3, 2020

knative-prow-robot requested review from evankanderson and rhuss August 3, 2020 15:04

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2020

knative-prow-robot reviewed Aug 3, 2020

View reviewed changes

knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 3, 2020

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 3, 2020

navidshaikh force-pushed the pr/channels branch from 524274e to c31e76a Compare August 3, 2020 15:23

navidshaikh force-pushed the pr/channels branch from 4919d45 to c853c40 Compare August 13, 2020 10:49

dsimansk reviewed Aug 13, 2020

View reviewed changes

pkg/kn/commands/channel/describe.go Outdated Show resolved Hide resolved

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 17, 2020

navidshaikh force-pushed the pr/channels branch 2 times, most recently from b822560 to 4bbf451 Compare August 18, 2020 08:04

navidshaikh added 2 commits August 18, 2020 14:46

Let channel reconcile, sleep for 5 secs after creation

7e08419

navidshaikh force-pushed the pr/channels branch from 4bbf451 to 7e08419 Compare August 18, 2020 09:16

navidshaikh commented Aug 18, 2020

View reviewed changes

maximilien suggested changes Aug 24, 2020

View reviewed changes

Add imcv1 and imcv1beta1 aliases

71c0edc

mattmoor reviewed Aug 25, 2020

View reviewed changes

rhuss reviewed Aug 25, 2020

View reviewed changes

Rename imcv1 alias to imc

027e80c

mattmoor reviewed Aug 25, 2020

View reviewed changes

rhuss reviewed Aug 25, 2020

View reviewed changes

knative-prow-robot assigned rhuss Aug 25, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2020

knative-prow-robot merged commit 5ed353d into knative:master Aug 25, 2020

navidshaikh deleted the pr/channels branch March 5, 2021 21:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(channel): Manage knative eventing channels #967

feat(channel): Manage knative eventing channels #967

navidshaikh commented Aug 3, 2020 •

edited

Loading

knative-prow-robot commented Aug 3, 2020

knative-prow-robot left a comment

navidshaikh commented Aug 3, 2020

navidshaikh commented Aug 17, 2020

navidshaikh commented Aug 17, 2020

navidshaikh commented Aug 17, 2020

navidshaikh commented Aug 17, 2020

navidshaikh Aug 18, 2020

navidshaikh Aug 25, 2020

rhuss Aug 25, 2020

rhuss Aug 25, 2020

navidshaikh commented Aug 18, 2020

maximilien left a comment

maximilien Aug 24, 2020

rhuss commented Aug 25, 2020

navidshaikh commented Aug 25, 2020

navidshaikh commented Aug 25, 2020

navidshaikh commented Aug 25, 2020

rhuss Aug 25, 2020

navidshaikh Aug 25, 2020

knative-metrics-robot commented Aug 25, 2020

navidshaikh commented Aug 25, 2020

rhuss left a comment

feat(channel): Manage knative eventing channels #967

feat(channel): Manage knative eventing channels #967

Conversation

navidshaikh commented Aug 3, 2020 • edited Loading

Description

Changes

Reference

TODO:

knative-prow-robot commented Aug 3, 2020

knative-prow-robot left a comment

Choose a reason for hiding this comment

Description

Changes

Reference

TODO:

navidshaikh commented Aug 3, 2020

navidshaikh commented Aug 17, 2020

navidshaikh commented Aug 17, 2020

navidshaikh commented Aug 17, 2020

navidshaikh commented Aug 17, 2020

navidshaikh Aug 18, 2020

Choose a reason for hiding this comment

navidshaikh Aug 25, 2020

Choose a reason for hiding this comment

rhuss Aug 25, 2020

Choose a reason for hiding this comment

rhuss Aug 25, 2020

Choose a reason for hiding this comment

navidshaikh commented Aug 18, 2020

maximilien left a comment

Choose a reason for hiding this comment

maximilien Aug 24, 2020

Choose a reason for hiding this comment

rhuss commented Aug 25, 2020

navidshaikh commented Aug 25, 2020

navidshaikh commented Aug 25, 2020

navidshaikh commented Aug 25, 2020

rhuss Aug 25, 2020

Choose a reason for hiding this comment

navidshaikh Aug 25, 2020

Choose a reason for hiding this comment

knative-metrics-robot commented Aug 25, 2020

navidshaikh commented Aug 25, 2020

rhuss left a comment

Choose a reason for hiding this comment

navidshaikh commented Aug 3, 2020 •

edited

Loading